Skip to content

GoodWe: Fix battery charging with PV when constraint is active#3509

Draft
iseeberg79 wants to merge 1 commit intoOpenEMS:developfrom
iseeberg79:fix/setpoint
Draft

GoodWe: Fix battery charging with PV when constraint is active#3509
iseeberg79 wants to merge 1 commit intoOpenEMS:developfrom
iseeberg79:fix/setpoint

Conversation

@iseeberg79
Copy link
Contributor

Problem

When an external controller (e.g., evcc) sets a discharge constraint via setActivePowerLessOrEquals(0) to keep the battery neutral, the GoodWe handler incorrectly charged the battery with available PV power instead of allowing PV to supply household consumption first.

Root Cause

In handleRemoteMode(), when activePowerSetPoint == 0 and PV was available, the condition pvProduction >= activePowerSetPoint (e.g., 300 >= 0) triggered CHARGE_BAT mode, routing PV to battery instead of household loads.

Fix

Added explicit handling for constraint case (activePowerSetPoint == 0) to return DISCHARGE_BAT, 0 (neutral), allowing PV to serve household consumption before any battery interaction.

Test Coverage

  • Fixed semantically incorrect test case
  • Added CHARGE_BAT test with proper PV surplus calculation
  • Added constraint test case (activePowerSetPoint == 0)
  • Added real-world evcc scenario test (GitHub evcc#22827)

Reference

@codecov
Copy link

codecov bot commented Jan 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #3509      +/-   ##
=============================================
+ Coverage      59.57%   59.63%   +0.06%     
  Complexity       105      105              
=============================================
  Files           3006     3006              
  Lines         130472   130474       +2     
  Branches        9639     9640       +1     
=============================================
+ Hits           77720    77789      +69     
+ Misses         49848    49770      -78     
- Partials        2904     2915      +11     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@iseeberg79
Copy link
Contributor Author

iseeberg79 commented Jan 8, 2026

@sfeilmeier, I would appreciate if you could review this. I've analyzed the issues related to evcc usage, referencing both the KACO implementation (https://community.openems.io/t/setactivepowerlessorequals-verhalten/10697/7) and the GoodWe (FENECON) implementation (already referenced). Sadly I've not found a solution for the KACO device as it looks like it's not about the implementation by OpenEMS (probably related to firmware/external library?). I cannot fiddle out the reason for PV routing for those.

It's related to the prevent discharge function implemented by the opemens template in evcc.

Because I have no hardware to test, you need to take it as theoretical work and someone else needs to the testing. It's additional unit tests are based on real-world scenarios and proove to solve the issue. Users having issues are FEMS users and because of this a test is not possible easily.

@iseeberg79
Copy link
Contributor Author

@sfeilmeier friendly reminder to discuss?

@Sn0w3y
Copy link
Collaborator

Sn0w3y commented Mar 5, 2026

Thanks for the analysis, but I believe the current behavior is actually correct.

activePowerSetPoint represents the desired AC output power of the ESS - it does not mean "battery power". When activePowerSetPoint == 0, it means: "The ESS should produce 0W on the AC side." If PV is producing 300W and 0W should go to AC, then charging the battery with the surplus 300W is the correct behavior. The PV energy has to go somewhere.

The existing logic in handleRemoteMode is consistent:

pvProduction (300) >= activePowerSetPoint (0) → CHARGE_BAT, pvProduction - activePowerSetPoint = 300

This means: "Feed activePowerSetPoint to AC, charge the battery with the rest of PV." That's exactly what should happen.

Your proposed fix (DISCHARGE_BAT, 0) would leave the 300W PV production unaccounted for - the behavior then depends entirely on how the GoodWe inverter internally handles that case, which is undefined.

If evcc wants to keep the battery neutral while still using PV for household consumption, the constraint needs to be expressed differently - this is not a bug in handleRemoteMode, but a semantic mismatch in how the constraint is set by the external controller.

@iseeberg79
Copy link
Contributor Author

iseeberg79 commented Mar 5, 2026

Thanks for your feedback. Yes, I realized from the discussion which started afterwards.

Let's not think about evcc here too much. Of course I'd appreciate a solution to successfully avoid discharging the battery on those external loads. It is clear to me that the use case needs to be resolved. That's another discussion ;-)

I would appreciate if somebody, best those who wrote the case, could clarify the reason for and take the proposal into account. I also see a dependency how the device is handling energy of course. Without a hardware and clear understanding about how it is supposed to get controlled this cannot be decided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants